refactor: alter Android API for loading the JS bundle from disk#207
Conversation
|
Hi @gronxb , FYI: after second thoughts, I think it will be better to expose the functionality with a second argument, |
|
I understand the intention behind this decision, which is to respect the existing parameters provided by React Native. However, I’d like to add one point: I think the interface exposed to users should remain consistent with the way things are configured in Values such as In a real greenfield environment, From this perspective, I think it would be more natural for ReactNativeBrownfield to unify the externally exposed interface under What do you think ? |
|
I agree with your reasoning, it's correct, but on the other hand, since loading from an arbitrary disk path may be security-concerning, then having a separate, verbose argument for that feature is a good option. I think this might be the reason why RN logic itself does not unify those 2 arguments into one, but just gives the path argument priority over the asset locator. For this reason, just to have an additive change that 1) does not surprise existing users and 2) explicitly states the value passed in can load an arbitrary JS bundle from disk, is better. This will allow us to release a patch version and have an easy adoption for ones that want to use this feature. There will be no 'surprises' that the existing arg does something unexpected. Also, while unifying to just one ar may make sense, on the other hand the arg is currently named |
89e3eaf to
60a8699
Compare
…pport a separate option
60a8699 to
c07598c
Compare
|
I understand, since breaking changes were also my biggest concern! Thank you. |
|
Awesome, I'm glad we reached the agreement! We'll try to ship this soon and let you know to test it from the published NPM package 🔧 |
|
Hi @gronxb - the newest version of |
Summary
This PR:
Test plan
CI green.